feat: inspector data: change and add data sources #284
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to change and add multiple data sources for inspector boundary configurations. The key enhancement is the ability to configure multiple data sources per boundary, each with their own columns and settings.
Changes:
- Added
idfield toInspectorBoundaryConfigschema for unique identification of boundary configurations - Created reusable
DataSourceSelectButtoncomponent with filtering by area set code - Enabled adding/removing/changing data sources in the inspector config tab
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/server/models/MapView.ts |
Added required id field to inspector boundary config schema |
migrations/1769099632447_boundary_config_add_id.ts |
Migration to add UUID id to existing boundary configs |
src/components/DataSourceItem.tsx |
Exported type DataSourceWithImportInfo for reuse |
src/app/map/[id]/components/DataSourceSelectButton.tsx |
New reusable component for data source selection with modal |
src/app/map/[id]/components/inspector/InspectorPanel.tsx |
Updated styling to fix overflow issues with min-h-0 |
src/app/map/[id]/components/inspector/InspectorDataTab.tsx |
Changed key from dataSourceId to id, added check for non-empty columns |
src/app/map/[id]/components/inspector/InspectorConfigTab.tsx |
Added functionality to add/remove data sources with UUID generation |
src/app/map/[id]/components/inspector/BoundaryConfigItem.tsx |
Added data source change/remove capabilities with area set filtering |
src/app/map/[id]/components/inspector/BoundaryDataPanel.tsx |
Added dataSourceId check to enabled condition |
src/app/map/[id]/components/controls/VisualisationPanel/VisualisationPanel.tsx |
Refactored to use new DataSourceSelectButton component |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const handleDataSourceIdChange = (dataSourceId: string) => { | ||
| setSelectedColumns([]); | ||
| onUpdate({ | ||
| ...boundaryConfig, | ||
| dataSourceId, | ||
| columns: [], | ||
| }); | ||
| }; |
There was a problem hiding this comment.
The local state configName is not updated when the data source is changed. This will cause the Name input field to display stale data from the previous data source until the user manually changes it. Consider calling setConfigName when the data source is changed.
| await sql` | ||
| UPDATE map_view | ||
| SET inspector_config = jsonb_set( | ||
| inspector_config, | ||
| '{boundaries}', | ||
| ( | ||
| SELECT jsonb_agg( | ||
| jsonb_set( | ||
| boundary, | ||
| '{id}', | ||
| to_jsonb(gen_random_uuid()::text) | ||
| ) | ||
| ) | ||
| FROM jsonb_array_elements(inspector_config->'boundaries') AS boundary | ||
| ) | ||
| ) | ||
| WHERE inspector_config ? 'boundaries' | ||
| AND jsonb_typeof(inspector_config->'boundaries') = 'array' | ||
| `.execute(db); |
There was a problem hiding this comment.
The migration may produce a null value when boundaries is an empty array, because jsonb_agg returns null when aggregating zero rows. Consider handling the empty array case by using COALESCE to preserve an empty array instead of setting it to null.
| export async function down(db: Kysely<any>): Promise<void> { | ||
| await sql` | ||
| UPDATE map_view | ||
| SET inspector_config = jsonb_set( | ||
| inspector_config, | ||
| '{boundaries}', | ||
| ( | ||
| SELECT jsonb_agg(boundary - 'id') | ||
| FROM jsonb_array_elements(inspector_config->'boundaries') AS boundary | ||
| ) | ||
| ) | ||
| WHERE inspector_config ? 'boundaries' | ||
| AND jsonb_typeof(inspector_config->'boundaries') = 'array' | ||
| `.execute(db); |
There was a problem hiding this comment.
The down migration may produce a null value when boundaries is an empty array, because jsonb_agg returns null when aggregating zero rows. Consider handling the empty array case by using COALESCE to preserve an empty array instead of setting it to null.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot add COALESCE to the up and down migrations to prevent null values in the array |
|
@joaquimds I've opened a new pull request, #285, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: joaquimds <12935136+joaquimds@users.noreply.github.com>
fix: prevent null boundaries in migration when array is empty
No description provided.